Skip to content

feat(import): extract and pass through executionRoleArn from starter toolkit YAML#680

Closed
jesseturner21 wants to merge 27 commits intoaws:mainfrom
jesseturner21:feat/import-execution-role
Closed

feat(import): extract and pass through executionRoleArn from starter toolkit YAML#680
jesseturner21 wants to merge 27 commits intoaws:mainfrom
jesseturner21:feat/import-execution-role

Conversation

@jesseturner21
Copy link
Copy Markdown
Contributor

@jesseturner21 jesseturner21 commented Mar 26, 2026

Summary

  • Parse the aws.execution_role field from .bedrock_agentcore.yaml and propagate it through the import pipeline into agentcore.json
  • Adds optional executionRoleArn to AgentEnvSpecSchema, ParsedStarterToolkitAgent, YAML parser, and toAgentEnvSpec()
  • Companion to CDK constructs PR: https://github.com/aws/agentcore-l3-cdk-constructs/pull/115

User Experience

Before (broken)

$ agentcore import --source .bedrock_agentcore.yaml -y
# ❌ execution_role from YAML is ignored
# ❌ agentcore.json has no executionRoleArn
# ❌ CDK creates a brand new IAM role, discarding the original

After (fixed)

$ cat .bedrock_agentcore.yaml
agents:
  my_agent:
    aws:
      execution_role: arn:aws:iam::123456789012:role/StarterToolkitRole
      ...

$ agentcore import --source .bedrock_agentcore.yaml -y
# ✅ CLI extracts execution_role from YAML

$ cat agentcore/agentcore.json
{
  "agents": [{
    "name": "my_agent",
    "executionRoleArn": "arn:aws:iam::123456789012:role/StarterToolkitRole",
    ...
  }]
}
# ✅ executionRoleArn written to agentcore.json
# ✅ CDK constructs (companion PR) will use this to import the role instead of creating a new one

Reverting to a CDK-managed role

Remove executionRoleArn from agentcore.json and run agentcore deploy — CDK will create a new role with standard policies.

Test plan

  • 4 new unit tests: YAML parsing with/without execution_role, spec generation with/without executionRoleArn
  • All existing 221 import tests pass
  • E2E: agentcore import writes correct executionRoleArn to agentcore.json

🤖 Generated with Claude Code

jesseturner21 and others added 27 commits March 26, 2026 09:09
Implements a 3-phase import flow that migrates Bedrock AgentCore Starter
Toolkit projects to the agentcore-cli:

- Phase 1: Creates companion CloudFormation resources (IAM roles/policies)
- Phase 2: Imports existing AWS resources via CFN IMPORT change set
- Phase 3: User runs `agentcore deploy` to reconcile the stack

Parses .bedrock_agentcore.yaml, scaffolds CLI project structure, copies
agent source code, publishes CDK assets, and adopts pre-existing runtimes
and memories into the CLI's CloudFormation stack.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove scaffold.ts; import now requires `agentcore create` first
- Resolve target from project's aws-targets.json (auto-select single,
  error on multiple without --target, create default from YAML if empty)
- Replace dangling Fn::GetAtt/Ref to removed primary resources with "*"
  in Phase 1 companion template (fixes IAM policy ARN validation)
- Fix memoryArn placeholder in deployed state (construct ARN from ID)
- Make --target optional (no default value)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Starter toolkit projects have multiple top-level directories (model/,
mcp_client/) which causes setuptools to fail with "Multiple top-level
packages discovered". Fix by appending [tool.setuptools] py-modules = []
to pyproject.toml after copy. Also run uv venv + uv sync so agentcore
dev works immediately after import.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Parse OAuth and API key credential providers from .bedrock_agentcore.yaml
identity config and add them to project.json during import. Fix YAML
parser to handle list items with nested key-value pairs (e.g.,
credential_providers). Fix eslint no-base-to-string errors by adding
explicit type casts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… mode from creating invalid memory

The simple YAML parser turns bare "mode:" (no value) into an empty object {},
which is truthy and passes the existing `!== 'NO_MEMORY' && memoryConfig.mode`
check. This caused a memory with mode={} to be created, which is invalid.
Added typeof check to ensure mode is actually a string before proceeding.

Also adds comprehensive unit tests for import memory handling (35 tests).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…oups

The YAML parser creates empty objects {} for keys with no list items
(e.g., `subnets:` with nothing underneath). The previous code used
`(networkModeConfig.subnets as string[]) ?? []` which doesn't protect
against this since {} is truthy. This resulted in subnets being {} instead
of [] when the YAML had empty subnet/security_group keys under VPC mode.

Now uses Array.isArray() to ensure we always get a proper array, falling
back to [] when the parser returns a non-array value.

Also adds comprehensive VPC import tests (18 tests covering parsing,
edge cases, mixed agents, quoted values, and starter toolkit format).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, running `agentcore import` twice with the same YAML would
attempt to re-import already-managed CloudFormation resources in Phase 2,
causing errors like "resource already exists in stack". The bug was that
agentsToImport and memoriesToImport were filtered from all parsed YAML
agents (regardless of whether they were skipped during merge), rather
than from only the newly-added ones.

Fix: track which agents/memories are actually added during the merge
step (newlyAddedAgentNames / newlyAddedMemoryNames) and filter
agentsToImport / memoriesToImport to only include those newly-added
resources.

Adds 17 unit tests covering first import, second import idempotency,
partial overlap, credential idempotency, and edge cases.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When importing a starter toolkit YAML with no physical IDs (agent_id and
memory_id are null), the import command would fail unnecessarily if no
deployment targets existed and the YAML had null account/region. This is
incorrect because the no-deploy path only needs config merge and source
copy -- no CloudFormation operations.

The fix computes hasPhysicalIds early and only performs strict target
resolution (requiring account/region) when physical IDs are present.
When no physical IDs exist, target resolution is lenient -- it uses an
existing target if available, or falls back to 'default' for stackName.

Also adds 22 unit tests covering the no-deploy path: YAML parsing of
null IDs, early return behavior, config merge, Python setup, absence of
CloudFormation operations, and target resolution edge cases.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
63 tests covering YAML parsing, toAgentEnvSpec conversion, merge logic,
source copy, Phase 1/Phase 2 template operations, findLogicalId helpers,
sanitize/toStackName, and integration for a single agent with no memory.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests cover:
- YAML parsing with 2, 3, and similar-named agents
- Memory deduplication across shared agents
- Credential extraction from identity.credential_providers
- findLogicalIdByProperty with multiple runtimes
- findLogicalIdByProperty Fn::Sub false substring match regression test
- filterCompanionOnlyTemplate with multiple primary resources
- buildImportTemplate with multiple import targets
- Stack name sanitization and source code directory structure
- Partial import (deployed + undeployed agents mixed)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit 25a1458125d9ecdae1dd3e8c2f6e17336fc7498f.
…enario

16 tests covering agent/memory/credential deduplication, Set-based name
matching, toMemorySpec clamping, source copy skip logic, append-only merge
behavior, and edge cases for projects with existing agents and memories.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…enario

16 tests covering agent/memory/credential deduplication, Set-based name
matching, toMemorySpec clamping, source copy skip logic, append-only merge
behavior, and edge cases for projects with existing agents and memories.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Publish CDK assets to S3 before Phase 1 (CodeBuild needs source zip)
- Handle directory assets in publishCdkAssets (zip before upload)
- Export publishCdkAssets for use in actions.ts
- Copy Dockerfile from starter toolkit config for Container builds
- Generate fallback Dockerfile if toolkit config not found
- Preserve previously imported resources during Phase 1 UPDATE
  by merging deployed template back into companion template

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change toCredentialSpec() to map OAuth providers to OAuthCredentialProvider
  instead of ApiKeyCredentialProvider, so the CLI wires CLIENT_ID/CLIENT_SECRET
  env vars correctly (not API_KEY)
- Make discoveryUrl optional in OAuthCredentialSchema for imported providers
  that already exist in Identity service
- Skip Identity service create/update for OAuth providers without discoveryUrl
  during deploy (provider already exists)
- Set AWS_REGION/AWS_DEFAULT_REGION to target.region before import operations
  to prevent region mismatch when env var differs from aws-targets.json

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n authorizer config

BUG-001: copyDirRecursive now skips symlinks and common excluded directories
(.venv, .git, __pycache__, node_modules, etc.) preventing EISDIR crash when
importing projects with virtualenvs containing symlinks like .venv/lib64 -> lib.

BUG-004: Import now warns when an agent has authorizer_configuration set in the
starter toolkit YAML, since custom JWT authorizer config is not automatically
imported and must be manually recreated via `agentcore add gateway`.

Constraint: CLI gateway model is separate from agent config, so full authorizer import requires design work
Rejected: Auto-create gateway during import | gateway-agent linking logic is complex and warrants its own feature
Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…containers

Two fixes:
1. Memory name prefix mismatch: CDK prefixes memory names with project
   name (e.g. "myproject_Agent_mem") but import searched for unprefixed
   YAML name. Added fallback lookup with project name prefix.
2. Container agents no longer trigger unnecessary Python venv setup
   during import, since dependencies are installed inside the Docker image.

Constraint: CDK BasePrimitiveConstruct generates physicalName as ${projectName}_${name}
Rejected: Stripping prefix from CDK names | would break other CDK conventions
Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
esbuild does not read tsconfig.json, so it defaults to classic JSX
transform (React.createElement) while tsconfig specifies react-jsx
(automatic). This produces 432 bare React.createElement calls in the
bundle that reference a nonexistent React global, crashing all TUI
commands (status, deploy, etc.) with "React is not defined".

Constraint: esbuild ignores tsconfig jsx settings by default
Rejected: Adding React as external | would require React in node_modules at runtime
Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Starter Toolkit injects memory ID via BEDROCK_AGENTCORE_MEMORY_ID,
but CDK constructs use MEMORY_{NAME}_ID pattern. After import, agent
code still references the old env var, causing memory to silently fail.

Add a yellow warning during import telling users the correct env var
name to update in their agent code.

Constraint: CDK env var naming is controlled by AgentCoreMemory.getEnvVarName()
Rejected: Auto-rewrite agent source code | too fragile across frameworks
Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Starter Toolkit injects memory ID via BEDROCK_AGENTCORE_MEMORY_ID,
but CDK constructs use MEMORY_{NAME}_ID pattern. After import, agent
code still references the old env var, causing memory to silently fail.

Show a git-diff-style warning during import with red/green highlighting
so users see exactly what to change in their agent code.

Constraint: CDK env var naming is controlled by AgentCoreMemory.getEnvVarName()
Rejected: Auto-rewrite agent source code | too fragile across frameworks
Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Import is a CLI-only command that requires --source flag and doesn't
have a TUI screen. Remove it from the interactive command picker.

Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Import fails with "The specified bucket does not exist" when the AWS
account hasn't been CDK-bootstrapped. The deploy command handles this
via useCdkPreflight, but import bypasses that since it's CLI-only.

Check bootstrap status after CDK synth and auto-bootstrap if needed,
before disposing the toolkit wrapper.

Tested on two unbootstrapped accounts (509471412906, 126432121770).

Constraint: Must bootstrap before disposing toolkitWrapper since bootstrap requires it
Rejected: Prompt user to manually run cdk bootstrap | poor UX for import flow
Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused waitUntilChangeSetCreateComplete import
- Prefix unused assetHash with underscore
- Add eslint-disable for necessary any cast in STS credentials
- Remove unnecessary type assertion in test
- Fix prettier formatting in multi-agent test

Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
OAuthCredentialProvider.discoveryUrl was made optional to support imported
providers that already exist in Identity service. Update tests to match:
- Schema test: expect success when discoveryUrl is omitted
- Pre-deploy identity tests: add discoveryUrl to fixtures that test
  update and error paths (without it, the code now skips the provider)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rAllMocks

checkBootstrapNeeded and bootstrapEnvironment were created as inline
vi.fn() inside the vi.mock factory. vi.clearAllMocks() in beforeEach
wipes their mockResolvedValue, causing handleImport to fail in CI
where test execution order differs from local runs.

Hoist the mocks and configure them in setupCommonMocks like all other
mock functions in the file.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…toolkit YAML

Parse the aws.execution_role field from .bedrock_agentcore.yaml and
propagate it through ParsedStarterToolkitAgent → toAgentEnvSpec →
agentcore.json so the CDK constructs can import the existing role
instead of creating a new one.

- Schema: add optional executionRoleArn to AgentEnvSpecSchema
- Types: add executionRoleArn to ParsedStarterToolkitAgent
- YAML parser: extract execution_role from aws config
- Actions: spread executionRoleArn into AgentEnvSpec

Confidence: high
Scope-risk: narrow

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@jesseturner21 jesseturner21 requested a review from a team March 26, 2026 16:53
@github-actions github-actions Bot added the size/xl PR size: XL label Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xl PR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant